Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(css_parser): implement CSS unicode range #3251

Merged
merged 7 commits into from
Jun 22, 2024
Merged

feat(css_parser): implement CSS unicode range #3251

merged 7 commits into from
Jun 22, 2024

Conversation

denbezrukov
Copy link
Contributor

@denbezrukov denbezrukov commented Jun 21, 2024 •
edited
Loading

Summary

Close #3172

See #3172 for details

  • error cases
  • changelog

Test Plan

cargo tedt

@github-actions github-actions bot added A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-CSS Language: CSS labels Jun 21, 2024
@denbezrukov denbezrukov changed the title feat(css_parser): Implement CSS unicode range feat(css_parser): implement CSS unicode range Jun 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 21, 2024 •
edited
Loading

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 50227 50227 0
Passed 49120 49120 0
Failed 1107 1107 0
Panics 0 0 0
Coverage 97.80% 97.80% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6534 6534 0
Passed 2084 2084 0
Failed 4450 4450 0
Panics 0 0 0
Coverage 31.89% 31.89% 0.00%

ts/babel

Test result main count This PR count Difference
Total 669 669 0
Passed 597 597 0
Failed 72 72 0
Panics 0 0 0
Coverage 89.24% 89.24% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18243 18243 0
Passed 13981 13981 0
Failed 4262 4262 0
Panics 0 0 0
Coverage 76.64% 76.64% 0.00%

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jun 21, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #3251 will improve performances by 6.88%

Comparing feat/css-unicode-range (ac2ddb1) with main (c502f6a)

Summary

⚡ 1 improvements
✅ 89 untouched benchmarks

Benchmarks breakdown

Benchmark main feat/css-unicode-range Change
⚡ jquery.min.js[cached] 28.7 ms 26.9 ms +6.88%

@github-actions github-actions bot added the A-Changelog Area: changelog label Jun 21, 2024
@denbezrukov denbezrukov marked this pull request as ready for review June 21, 2024 19:46
togami2864
togami2864 approved these changes Jun 22, 2024
View reviewed changes
Copy link
Contributor

@togami2864 togami2864 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost LGTM

crates/biome_css_analyze/tests/specs/nursery/noUnknownUnit/invalid.css.snap

25 │ a { background: /* comment */ image-set('img1x.png' 1x, 'img2x.png' 2x) left 20x / 15% 60% repeat-x; }
26 │ a { background-image: image-set('img1x.png' 1pix, 'img2x.png' 2x); }
> 27 │ @font-face { color: U+0100-024F; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to consider how to handle such an invalid css in the future...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that it could be a rule.
Like noUnknownUnicodeRange or something?

crates/biome_css_parser/src/syntax/property/unicode_range.rs Outdated Show resolved Hide resolved
@denbezrukov denbezrukov merged commit 6ad7fbf into main Jun 22, 2024
16 checks passed
@denbezrukov denbezrukov deleted the feat/css-unicode-range branch June 22, 2024 13:54
@denbezrukov denbezrukov mentioned this pull request Jun 24, 2024
Merged
@ramzimalhas ramzimalhas mentioned this pull request Aug 21, 2024
Open
This was referenced Aug 28, 2024
Open
Open
This was referenced Sep 9, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@togami2864 togami2864 togami2864 approved these changes

Assignees
No one assigned
Labels
A-Changelog Area: changelog A-Formatter Area: formatter A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐛 CSS parsing and Unicode point ranges
2 participants
@denbezrukov @togami2864